Skip to content

feat: add metadata updates and requirements interface #257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Oct 17, 2025
Merged

feat: add metadata updates and requirements interface #257

merged 14 commits into from
Oct 17, 2025

Conversation

@gty404
Copy link
Contributor

@gty404 gty404 commented Oct 12, 2025

This commit introduces the metadata update and update requirements system for table metadata modifications.

@gty404 gty404 force-pushed the table-metadata branch 5 times, most recently from b147683 to d76a780 Compare October 12, 2025 07:51
HuaHuaY
HuaHuaY reviewed Oct 13, 2025
View reviewed changes
wgtmac
wgtmac reviewed Oct 14, 2025
View reviewed changes
HuaHuaY
HuaHuaY reviewed Oct 15, 2025
View reviewed changes
wgtmac
wgtmac requested changes Oct 15, 2025
View reviewed changes
@gty404 gty404 changed the title feat: implement metadata updates and update requirements feat: add metadata updates and requirements interface Oct 15, 2025
zhjwpku
zhjwpku approved these changes Oct 16, 2025
View reviewed changes
Copy link
Collaborator

@zhjwpku zhjwpku left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was initially confused about renaming MetadataUpdate to TableUpdate and UpdateRequirement to TableRequirement, but after checking the iceberg-rust and iceberg-python implementations, it now makes sense to me.

@wgtmac
Copy link
Member

wgtmac commented Oct 16, 2025

While I know both iceberg-python and iceberg-rust name them as TableUpdate and TableRequirement. However, it might looks weird to use Table prefix in terms of dealing with updates to table, view and perhaps the in-discussion materialized view.

WDYT? @Fokko @liurenjie1024

@liurenjie1024
Copy link

liurenjie1024 commented Oct 16, 2025

While I know both iceberg-python and iceberg-rust name them as TableUpdate and TableRequirement. However, it might looks weird to use Table prefix in terms of dealing with updates to table, view and perhaps the in-discussion materialized view.

WDYT? @Fokko @liurenjie1024

Thanks @wgtmac for pinging me. I prefer to add an prefix to improve readability.

wgtmac
wgtmac reviewed Oct 17, 2025
View reviewed changes
Copy link
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. I left some minor comments.

@wgtmac
Copy link
Member

wgtmac commented Oct 17, 2025

Thanks @liurenjie1024 for the input! I think we should stick with the rest catalog api to use TableUpdate, ViewUpdate, etc.

gty404 added 10 commits October 17, 2025 11:35
@gty404
feat: implement metadata updates and update requirements
d644977 
This commit introduces the metadata update and update requirements
system for table metadata modifications.
@gty404
Fix the string comparison issue
a1c800c
@gty404
fix comments
994741c
@gty404
fix comments
abe3053
@gty404
fix comments
a1411d8
@gty404
fix build error
f040c01
@gty404
feat: implement metadata updates and update requirements
d8a736e 
This commit introduces the metadata update and update requirements
system for table metadata modifications.
@gty404
fix comments
e92f45c
@gty404
rename some class name and fix comments
7f1b7d9
@gty404
fix conflict
4439f8d
wgtmac
wgtmac approved these changes Oct 17, 2025
View reviewed changes
Copy link
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've just left a comment w.r.t. class names.

@wgtmac wgtmac merged commit b2b38af into apache:main Oct 17, 2025
10 checks passed
@gty404 gty404 deleted the table-metadata branch October 17, 2025 14:03
@smaheshwar-pltr smaheshwar-pltr mentioned this pull request Oct 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wgtmac wgtmac wgtmac approved these changes

+3 more reviewers

@HuaHuaY HuaHuaY HuaHuaY left review comments

@nullccxsy nullccxsy nullccxsy left review comments

@zhjwpku zhjwpku zhjwpku approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@gty404 @wgtmac @liurenjie1024 @zhjwpku @HuaHuaY @nullccxsy